Account for common `impl Trait`/`dyn Trait` return type errors #68195

estebank · 2020-01-14T02:30:45Z

When all return paths have the same type, suggest impl Trait.
When all return paths implement the expected trait, suggest Box<dyn Trait> and mention using an enum.
When multiple different types are returned and impl Trait is expected, extend the explanation.
When return type is impl Trait and the return paths do not implement Trait, point at the returned values.
Split src/librustc/traits/error_reporting.rs into multiple files to keep size under control.

Fix #68110, cc #66523.

rust-highfive · 2020-01-14T02:30:48Z

Some changes occurred in diagnostic error codes

cc @GuillaumeGomez

rust-highfive · 2020-01-14T02:30:50Z

r? @eddyb

(rust_highfive has picked a reviewer for you, use r? to override)

estebank · 2020-01-14T02:32:26Z

The changes have reasonable single commit sizes, but the final diff is on the larger side.

CC @rust-lang/wg-diagnostics

src/librustc_typeck/check/coercion.rs

estebank · 2020-01-14T02:40:46Z

Note that suggestions always come last, which messes up the flow a bit (I'd prefer the last thing in the diagnostic to be the links, but alas I won't be changing DiagnosticBuilder today).

src/librustc/traits/error_reporting/suggestions.rs

src/test/ui/error-codes/E0746.stderr

oli-obk · 2020-01-14T08:03:23Z

r? @oli-obk

src/librustc/traits/error_reporting.rs

src/librustc_typeck/check/coercion.rs

src/librustc/traits/error_reporting.rs

src/test/ui/const-generics/array-impls/into-iter-no-impls-length-33.stderr

src/librustc/traits/error_reporting/suggestions.rs

src/test/ui/impl-trait/dyn-trait-return-should-be-impl-trait.stderr

src/librustc_error_codes/error_codes/E0746.md

src/librustc/traits/mod.rs

src/librustc/traits/error_reporting/suggestions.rs

oli-obk · 2020-01-15T09:17:04Z

r? @Centril (reflecting reality here)

estebank · 2020-01-15T19:09:02Z

I'm looking at addressing the cases where T is not object safe to keep it out of the suggestion, but it is a hard error that doesn't even reach the unsized error:

error[E0038]: the trait `NotObjectSafe` cannot be made into an object
  --> src/test/ui/impl-trait/object-unsafe-trait-in-return-position-dyn-trait.rs:21:1
   |
3  |     fn foo() -> Self;
   |        --- associated function `foo` has no `self` parameter
...
21 | fn car() -> dyn NotObjectSafe {
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `NotObjectSafe` cannot be made into an object

error[E0038]: the trait `NotObjectSafe` cannot be made into an object
  --> src/test/ui/impl-trait/object-unsafe-trait-in-return-position-dyn-trait.rs:28:1
   |
3  |     fn foo() -> Self;
   |        --- associated function `foo` has no `self` parameter
...
28 | fn cat() -> Box<dyn NotObjectSafe> {
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `NotObjectSafe` cannot be made into an object

The impl T case doesn't account for object safety yet.

src/librustc_typeck/check/coercion.rs

Suggest `impl Trait` when possible, and `Box<dyn Trait>` otherwise.

When a type error involves a `dyn Trait` as the return type, do not emit the type error, as the "return type is not `Sized`" error will provide enough information to the user.

Centril · 2020-01-16T21:37:24Z

@bors r+

bors · 2020-01-16T21:37:25Z

📌 Commit 029a9c6 has been approved by Centril

Account for common `impl Trait`/`dyn Trait` return type errors - When all return paths have the same type, suggest `impl Trait`. - When all return paths implement the expected `trait`, suggest `Box<dyn Trait>` and mention using an `enum`. - When multiple different types are returned and `impl Trait` is expected, extend the explanation. - When return type is `impl Trait` and the return paths do not implement `Trait`, point at the returned values. - Split `src/librustc/traits/error_reporting.rs` into multiple files to keep size under control. Fix rust-lang#68110, cc rust-lang#66523.

@ghost

Rollup of 6 pull requests Successful merges: - #67956 (Detail transitive containment in E0588 diagnostic) - #68153 (resolve: Point at the private item definitions in privacy errors) - #68195 (Account for common `impl Trait`/`dyn Trait` return type errors) - #68288 (Fix some of the rustfmt fallout in Miri) - #68292 (don't clone types that are copy) - #68301 (Don't propagate __RUST_TEST_INVOKE to subprocess) Failed merges: r? @ghost

Ixrec · 2020-01-20T14:38:03Z

Do we have an issue for changing the "return type cannot be a bare trait" language to something more dyn-aware? (I couldn't find one)

estebank · 2020-01-20T19:03:58Z

It should be "unboxed trait object". Don't think there's a ticket.

Further improve `impl Trait`/`dyn Trait` suggestions After reading [_Returning Trait Objects_ by Bryce Fisher-Fleig](https://bryce.fisher-fleig.org/blog/returning-trait-objects/), [I noticed that](https://www.reddit.com/r/rust/comments/esueur/returning_trait_objects/ffczl4k/) rust-lang#68195 had a few bugs due to not ignoring `ty::Error`. - Account for `ty::Error`. - Account for `if`/`else` and `match` blocks when pointing at return types and referencing their types. - Increase the multiline suggestion output from 6 lines to 20.

Further improve `impl Trait`/`dyn Trait` suggestions After reading [_Returning Trait Objects_ by Bryce Fisher-Fleig](https://bryce.fisher-fleig.org/blog/returning-trait-objects/), [I noticed that](https://www.reddit.com/r/rust/comments/esueur/returning_trait_objects/ffczl4k/) #68195 had a few bugs due to not ignoring `ty::Error`. - Account for `ty::Error`. - Account for `if`/`else` and `match` blocks when pointing at return types and referencing their types. - Increase the multiline suggestion output from 6 lines to 20.

rust-highfive assigned eddyb Jan 14, 2020

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jan 14, 2020

estebank commented Jan 14, 2020

View reviewed changes

src/librustc_typeck/check/coercion.rs Outdated Show resolved Hide resolved

estebank force-pushed the impl-trait-2000 branch from 9498bc0 to 7f95375 Compare January 14, 2020 02:36

estebank commented Jan 14, 2020

View reviewed changes

src/librustc/traits/error_reporting/suggestions.rs Show resolved Hide resolved

This comment has been minimized.

Sign in to view

dtolnay reviewed Jan 14, 2020

View reviewed changes

src/test/ui/error-codes/E0746.stderr Outdated Show resolved Hide resolved

rust-highfive assigned oli-obk and unassigned eddyb Jan 14, 2020

Centril reviewed Jan 14, 2020

View reviewed changes

Centril self-assigned this Jan 14, 2020

estebank force-pushed the impl-trait-2000 branch from 6ed3e32 to 8ed4fc8 Compare January 14, 2020 21:27

This comment has been minimized.

Sign in to view

Centril reviewed Jan 15, 2020

View reviewed changes

src/librustc/traits/error_reporting/suggestions.rs Outdated Show resolved Hide resolved

Centril reviewed Jan 15, 2020

View reviewed changes

src/test/ui/impl-trait/dyn-trait-return-should-be-impl-trait.stderr Outdated Show resolved Hide resolved

Centril reviewed Jan 15, 2020

View reviewed changes

rust-highfive unassigned oli-obk Jan 15, 2020

This comment has been minimized.

Sign in to view

estebank force-pushed the impl-trait-2000 branch from e09d187 to 1a686f9 Compare January 15, 2020 23:56

estebank commented Jan 15, 2020

View reviewed changes

src/librustc_typeck/check/coercion.rs Show resolved Hide resolved

This comment has been minimized.

Sign in to view

estebank added 2 commits January 16, 2020 09:37

Specific error for unsized dyn Trait return type

6fd5641

Suggest `impl Trait` when possible, and `Box<dyn Trait>` otherwise.

Account for diverging types in return impl Trait

75eabb1

estebank added 12 commits January 16, 2020 09:37

Elide E0308 errors in favor of E0746

ea7e885

When a type error involves a `dyn Trait` as the return type, do not emit the type error, as the "return type is not `Sized`" error will provide enough information to the user.

Make impl Trait suggestion in E0746 MachineApplicable

b4bbe78

When trait bounds are missing for return values, point at them

e1dd8a9

fmt

93293c5

Add E0746 explanation to the index

4c13d25

Split librustc/traits/error_reporting.rs

00c8272

fix error code index comment

4a75ef9

review comments

5b36c18

review comments

509cb33

Fix error index test

c305ac3

review comments

d7a6212

Account for object safety when suggesting Box<dyn Trait>

00e2626

estebank force-pushed the impl-trait-2000 branch from 1a686f9 to 00e2626 Compare January 16, 2020 18:46

review comments

029a9c6

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 16, 2020

Dylan-DPC-zz mentioned this pull request Jan 17, 2020

Rollup of 6 pull requests #68305

Merged

bors merged commit 029a9c6 into rust-lang:master Jan 17, 2020

estebank mentioned this pull request Jan 24, 2020

Further improve impl Trait/dyn Trait suggestions #68522

Merged

estebank mentioned this pull request Feb 4, 2020

Confusing error message with panic!() and impl trait #66523

Closed

estebank deleted the impl-trait-2000 branch November 9, 2023 05:17

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Account for common `impl Trait`/`dyn Trait` return type errors #68195

Account for common `impl Trait`/`dyn Trait` return type errors #68195

estebank commented Jan 14, 2020 •

edited

Loading

rust-highfive commented Jan 14, 2020

rust-highfive commented Jan 14, 2020

estebank commented Jan 14, 2020

estebank commented Jan 14, 2020

This comment has been minimized.

oli-obk commented Jan 14, 2020

This comment has been minimized.

oli-obk commented Jan 15, 2020

estebank commented Jan 15, 2020

This comment has been minimized.

This comment has been minimized.

Centril commented Jan 16, 2020

bors commented Jan 16, 2020

Ixrec commented Jan 20, 2020

estebank commented Jan 20, 2020

Account for common impl Trait/dyn Trait return type errors #68195

Account for common impl Trait/dyn Trait return type errors #68195

Conversation

estebank commented Jan 14, 2020 • edited Loading

rust-highfive commented Jan 14, 2020

rust-highfive commented Jan 14, 2020

estebank commented Jan 14, 2020

estebank commented Jan 14, 2020

This comment has been minimized.

oli-obk commented Jan 14, 2020

This comment has been minimized.

oli-obk commented Jan 15, 2020

estebank commented Jan 15, 2020

This comment has been minimized.

This comment has been minimized.

Centril commented Jan 16, 2020

bors commented Jan 16, 2020

Ixrec commented Jan 20, 2020

estebank commented Jan 20, 2020

Account for common `impl Trait`/`dyn Trait` return type errors #68195

Account for common `impl Trait`/`dyn Trait` return type errors #68195

estebank commented Jan 14, 2020 •

edited

Loading